-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
linkcheck: Allow case-insensitive URL comparisons #14046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
linkcheck: Allow case-insensitive URL comparisons #14046
Conversation
|
Hi @FazeelUsmani - thank you for developing and describing this pull request. I have a concern that enabling the option reduces the precision of other hyperlinks that are checked. Could you explain the use case where it would be easier for a documentation project to enable this option by editing the |
|
That’s a good point, @jayaddison. |
|
@FazeelUsmani got it, understood. As often happens, I had a misunderstanding to begin with - you are saying that this only affects whether case-adjusted response URLs are considered to be Let me think about this a little more; I do understand the value in this now, but am wary of (and trying to think of) any problem side-effects. |
|
(also, thank you for the explanation) |
|
Separately: I do think that we should probably isolate the |
|
Hmm.. makes sense. I can refactor this into two separate options: This would give users more granular control. Most users would likely want |
|
Two options seems overkill for this use-case. What do browsers do de facto on case mismatches on fragment IDs? A |
|
Fair point — browsers generally treat fragment IDs as case-sensitive, though behavior can vary depending on the HTML generator. My thought was mainly to avoid false negatives in edge cases (like auto-generated anchors that normalize casing differently). |
|
I can't think of drawbacks to the The anchor-checking I'm less certain about; given that we believe browsers seem to navigate to anchors case-sensitively -- something I too checked locally and that is certainly the case in Firefox 140.4 -- I'd be reluctant to offer that without a demonstrable use-case (again that can't be solved easily by fixing the source documentation). |
|
That makes sense — I’ll keep it as a single |
|
Sounds good to me! Thanks @FazeelUsmani. |
56d6a63 to
d115b1e
Compare
jayaddison
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature implementation looks good to me; thank you @FazeelUsmani!
|
Hi @jayaddison, |
You're welcome. I think the best response I can offer to answer that -- and I think you have followed most/all of the guidance there already -- is to refer to the Sphinx official contributing guide: https://www.sphinx-doc.org/en/master/internals/contributing.html#contribute-code |
|
@jayaddison Done! I've implemented all three cleanup suggestions |
| @@ -0,0 +1,3 @@ | |||
| `path1 <http://localhost:7777/path1>`_ | |||
|
|
|||
| `path2 <http://localhost:7777/path2>`_ | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
That looks good to me - much neater, I think! Thanks again. |
Co-authored-by: James Addison <[email protected]>
…sphinx into linkcheck_case_insensitive
jayaddison
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great - thank you, @FazeelUsmani!
|
Thanks @jayaddison @FazeelUsmani. I've pushed some tweaks to the implementation & renamed the config option to A |
|
A note that the That surprised me - I felt like those tests should run pretty quickly. Other linkcheck unit tests aren't exactly speedy either (~1s to ~2s is not atypical), but these ones seem noticeably longer duration. |
|
...hmm; maybe those longer durations are particularly present when running in GitHub CI on Windows. I'll try to determine whether that's the case, and more about it generally, soon. |
My current best-guess is that these extended test durations are due to the If so, that would mean that the cause of the long-duration tests is not really related to the logic in this pull request; it's simply a side-effect of the fact that these tests were added below the socket timeout config adjustment. We could test this theory by relocating |
This PR adds a new configuration option
linkcheck_ignore_caseto enable case-insensitive URL and anchor checking in the linkcheck builder.Problem
Some web servers (e.g., GitHub, certain hosting platforms) are case-insensitive and may return URLs with different casing than the original link. This causes the linkcheck builder to report false-positive redirects when the URLs differ only in case, even though they point to the same resource.
Solution
linkcheck_ignore_caseboolean configuration option (default:False)AnchorCheckParserto support case-insensitive matching when enableddoc/usage/configuration.rst